-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[MXNET-995] Constant Initializer for ND Array #12677
Conversation
@mxnet-label-bot [pr-awaiting-review] |
@@ -2479,6 +2479,8 @@ def array(source_array, ctx=None, dtype=None): | |||
if isinstance(source_array, NDArray): | |||
dtype = source_array.dtype if dtype is None else dtype | |||
else: | |||
if isinstance(source_array, (float, int)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mx.base.numeric_types is better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my concern here is that you implicitly converted 0-d array(constant) to 1-d array here, which may not be very appropriate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, my logic was since MXNet NDArray needs a 1d array minimum, I should convert constant. Else, is there a way I can do constant initialization support? Would be great to get some help in that direction.
@@ -869,7 +869,7 @@ def _sync_copyfrom(self, source_array): | |||
source_array = np.ascontiguousarray(source_array, dtype=self.dtype) | |||
if source_array.shape != self.shape: | |||
raise ValueError('Shape inconsistent: expected %s vs got %s'%( | |||
str(self.shape), str(source_array.shape))) | |||
str(source_array.shape), str(self.shape))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why switch the order of expected vs got?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at #12676
I found it to be odd. Hence created an issue and fixed it in this PR
Don't you think so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhreshold could you confirm if the above issue (#12676 ) is right or not? coz the value error it gave sounded confusing due to mismatched shapes. Thanks
Please check that this case was resolved by PR #12678 too. |
Ah, alright. So I'll take mine down then. Thanks for your contribution. |
Description
Allows 1D (constant) to be used to initialize an NDArray or Symbol
Fixes #12672 and #12676
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes